fix: use environment variables to check for terminal #2330
Conversation
Issue mobile-dev-inc#2278 finds CLibrary.isatty gives an error unless both the --no-color and --no-ansi flags are given. This fix switches to environment variables because on CachyOS Linux and Fedora Linux users consistently got those errors.
| val stdoutIsTTY = CLibrary.isatty(CLibrary.STDOUT_FILENO) != 0 | ||
| // Instead of using CLibrary.isatty, use environment variables to detect terminal | ||
| val forceDisable = System.getenv("MAESTRO_DISABLE_ANSI")?.toBoolean() ?: false | ||
| val isTTY = !forceDisable && System.getenv("TERM") != null |
There was a problem hiding this comment.
I absolutely love the idea of dropping a dependency and simplifying the solution.
This will work on a Mac, but will this work on Windows?
There was a problem hiding this comment.
From my research, it looks like the answer is no, it does not. I have added a few more checks specific for windows based on my research, but the only one I have actually been able to verify returns a boolean is ($PSVersionTable -ne $null). I ran into issues building on my first Windows machine (seemed to be a problem with Java, somehow, but I could not figure it out).
I am working on getting my other Windows machine ready to build it and see if I can verify any of it works.
Update: Will have to continue later or someone else can try to get my fix-cli branch working on a Windows machine. I did find I had to specifically install Java 11. For later reference, I have included the error from my most recent build.
Output from failed build
> Task :maestro-ai:compileKotlin
w: file:///C:/Users/mcgen/Maestro/maestro-ai/src/main/java/maestro/ai/DemoApp.kt:106:13 Variable 'aiClient' is never used
w: file:///C:/Users/mcgen/Maestro/maestro-ai/src/main/java/maestro/ai/anthropic/Client.kt:54:13 Variable 'actualImageDetail' is never used
> Task :maestro-studio:web:deps FAILED
> Task :maestro-utils:compileKotlin
w: file:///C:/Users/mcgen/Maestro/maestro-utils/src/main/kotlin/Metrics.kt:27:28 Type mismatch: inferred type is String? but String was expected
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':maestro-studio:web:deps'.
> A problem occurred starting process 'command 'npm.cmd''
Issue #2278 finds CLibrary.isatty gives an error when any
maestrocommand is run unless both the --no-color and --no-ansi flags are given. This fix switches to environment variables because on CachyOS Linux and Fedora Linux users consistently got those errors.Proposed changes
I switched from using the CLibrary.isatty to using environment variables to check if the call is from a terminal.
Based on the issue, it seems this may be a Linux specific problem. If the team would rather solve this another way, I am happy to help if you would like to direct me how you want it solved. Otherwise, if the team would prefer to go another direction without my help, that is also understandable.
Testing
I ran the
./maestro-cli/build/install/maestro/bin/maestrocommand with no flags and got expected output.I also got the demo_app in the
e2e/directory installed on an Android Emulator and ran the tests provided. The output is below.Ran the
./maestro-cli/build/install/maestro/bin/maestro --device emulator-5554 test e2e/workspaces/demo_appcommand.Output from tests
Issues fixed
Possible fix for Issue #2278
Feedback welcome
I am new to Maestro, so I would appreciate any feedback on how I solved the issue, if I should delete the comments in the code, and anything else that would help the team or project better.